Skip to content

Add cache level limits to AddCacheInfo#190

Closed
wamuir wants to merge 1 commit into
google:masterfrom
wamuir:add-cache-level-limits
Closed

Add cache level limits to AddCacheInfo#190
wamuir wants to merge 1 commit into
google:masterfrom
wamuir:add-cache-level-limits

Conversation

@wamuir

@wamuir wamuir commented Oct 26, 2021

Copy link
Copy Markdown

list_cpu_features segfaults in at least some cases after 641e24e. I am experiencing on Linux (Comet Lake) and FreeBSD (Kaby Lake). PR resolves by limiting parsing in AddCacheInfo to the range of possible cache levels.

@gchatelet

Copy link
Copy Markdown
Collaborator

Thx for the PR, I spotted it this morning as well.
Can you simply change the for loop as follows (it will prevent unpleasant merging on my side)

  for (int cache_id = 0; cache_id < CPU_FEATURES_MAX_CACHE_LEVEL &&
                         info->size < CPU_FEATURES_MAX_CACHE_LEVEL;
       cache_id++) {

It's still not a proper fix but at least it will fix the corruption.

@gchatelet

Copy link
Copy Markdown
Collaborator

Ha my bad I misread your patch.
The problem is not in list_cpu_features.c but in cpuinfo_x86.c.

I'll send a fix ASAP

@gchatelet gchatelet closed this Oct 26, 2021
@wamuir

wamuir commented Oct 26, 2021

Copy link
Copy Markdown
Author

Awesome, thanks!

gchatelet added a commit that referenced this pull request Oct 26, 2021
@gchatelet gchatelet self-assigned this Oct 26, 2021
@gchatelet gchatelet added the bug Something isn't working label Oct 26, 2021
@gchatelet

Copy link
Copy Markdown
Collaborator

The memory corruption is fixed but cache info might be duplicated (read from Leaf2 and Leaf 4/0x8000001D).
I'll prepare a proper fix ASAP.

gchatelet added a commit that referenced this pull request Oct 26, 2021
@gchatelet gchatelet added this to the v0.7.0 milestone Mar 8, 2022
EylonKrause added a commit to EylonKrause/cpu_features that referenced this pull request Jun 30, 2026
…_LEVEL

ParseLeaf2 appends a CacheLevelInfo to CacheInfo::levels (a fixed
CPU_FEATURES_MAX_CACHE_LEVEL=10 array) for every non-zero leaf-2
descriptor byte, but the loop is bounded only by sizeof(data)==16 and
never checks info->size against the array capacity. A CPUID leaf 2
advertising more than 10 non-zero descriptors (up to 15 are possible)
writes past levels[10] -- an out-of-bounds write reachable from public
GetX86CacheInfo() on hardware/hypervisor-controlled CPUID input. The
sibling ParseCacheInfo already guards its loop with
'info.size < CPU_FEATURES_MAX_CACHE_LEVEL' (added for the same class of
corruption in google#190); apply the same bound to ParseLeaf2.

Adds a regression test (Leaf2_TooManyDescriptors_DoesNotOverflow) that
feeds a leaf 2 with 15 descriptors: it triggers an ASan
stack-buffer-overflow before the fix and passes after.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants